Fix chinese input and text pasting#1479
Conversation
fb02c73 to
4c9a20b
Compare
Reviewer's GuideAdds proper Wayland text input/IME focus handling and clipboard synchronization so plugin Wayland clients can use Chinese input and paste text correctly, primarily by wiring Qt’s private text input protocols to seat focus changes and ensuring the correct QQuickItem gains active focus. Sequence diagram for IME focus and Chinese input forwardingsequenceDiagram
actor User
participant PluginClient
participant PluginSurface
participant PluginTextInput
participant QWaylandSeat
participant PluginManager
participant TextInputExtensions
participant QWaylandQuickItem
participant QInputMethod
User->>PluginClient: Clicks input field
PluginClient->>PluginSurface: Give keyboard focus
PluginClient->>PluginTextInput: text_input.enable
PluginSurface->>QWaylandSeat: Keyboard focus change
QWaylandSeat-->>PluginManager: keyboardFocusChanged(newFocus)
PluginManager->>PluginSurface: newFocus.updateSelection()
PluginManager->>TextInputExtensions: Iterate seat.extensions()
Note over TextInputExtensions,PluginManager: For each extension
TextInputExtensions-->>PluginManager: QWaylandTextInput
PluginManager->>TextInputExtensions: QWaylandTextInputPrivate.setFocus(newFocus)
PluginManager->>TextInputExtensions: connect(surfaceEnabled, onTextInputSurfaceEnabled)
TextInputExtensions-->>PluginManager: QWaylandTextInputV3
PluginManager->>TextInputExtensions: QWaylandTextInputV3Private.setFocus(newFocus)
PluginManager->>TextInputExtensions: connect(surfaceEnabled, onTextInputSurfaceEnabled)
TextInputExtensions-->>PluginManager: QWaylandQtTextInputMethod
PluginManager->>TextInputExtensions: QWaylandQtTextInputMethodPrivate.inputPanelVisible = true
PluginManager->>TextInputExtensions: WlQtTextInputMethodHelper.setFocusCustom(newFocus)
PluginManager->>TextInputExtensions: connect(surfaceEnabled, onTextInputSurfaceEnabled)
PluginTextInput-->>TextInputExtensions: surfaceEnabled(PluginSurface)
TextInputExtensions-->>PluginManager: surfaceEnabled(PluginSurface)
PluginManager->>PluginManager: onTextInputSurfaceEnabled(surface)
PluginManager->>PluginSurface: surface.views()
PluginSurface-->>PluginManager: view with QWaylandQuickItem
PluginManager->>QWaylandQuickItem: forceActiveFocus
User->>QInputMethod: Type Chinese text
QInputMethod-->>QWaylandQuickItem: QInputMethodEvent
QWaylandQuickItem-->>TextInputExtensions: QWaylandInputMethodControl.inputMethodEvent
TextInputExtensions-->>PluginTextInput: commit_string
PluginTextInput-->>PluginClient: Insert committed Chinese text
Sequence diagram for clipboard synchronization and pastesequenceDiagram
actor User
participant HostApp
participant HostClipboard
participant PluginManager
participant QWaylandCompositor
participant PluginClient
User->>HostApp: Copy text
HostApp->>HostClipboard: Set clipboard data
HostClipboard-->>PluginManager: changed(mode)
PluginManager->>HostClipboard: mimeData(mode)
HostClipboard-->>PluginManager: QMimeData
PluginManager->>QWaylandCompositor: setRetainedSelectionEnabled(true) (init)
PluginManager->>QWaylandCompositor: overrideSelection(mimeData)
QWaylandCompositor-->>PluginClient: Update Wayland selection
User->>PluginClient: Paste
PluginClient->>QWaylandCompositor: Request selection data
QWaylandCompositor-->>PluginClient: Send clipboard contents
Class diagram for updated PluginManager text input handlingclassDiagram
class PluginManager {
+PluginManager(QWaylandCompositor* compositor)
+void initialize()
+void setupMouseFocusListener()
+void setupTextInputProxy(QWaylandCompositor* compositor)
+void onTextInputSurfaceEnabled(QWaylandSurface* surface)
}
class WlQtTextInputMethodHelper {
+static void setFocusCustom(QWaylandQtTextInputMethodPrivate* d, QWaylandSurface* surface)
}
class QWaylandCompositor {
+QWaylandSeat* defaultSeat()
+void setRetainedSelectionEnabled(bool enabled)
+void overrideSelection(const QMimeData* mimeData)
}
class QWaylandSeat {
+QList~QObject*~ extensions()
+Q_SIGNAL void keyboardFocusChanged(QWaylandSurface* newFocus, QWaylandSurface* oldFocus)
}
class QWaylandSurface {
+QList~QWaylandView*~ views()
+void updateSelection()
+QWaylandClient* client()
+QtWaylandServer::wl_surface* resource()
}
class QWaylandView {
+QQuickItem* renderObject()
}
class QWaylandQuickItem {
+void forceActiveFocus(Qt::FocusReason reason)
}
class QWaylandTextInputPrivate {
+void setFocus(QWaylandSurface* surface)
QWaylandSurface* focusedSurface
}
class QWaylandTextInputV3Private {
+void setFocus(QWaylandSurface* surface)
QWaylandSurface* focusedSurface
}
class QWaylandQtTextInputMethodPrivate {
QWaylandSurface* focusedSurface
QtWaylandServer::qt_text_input_method_v1::Resource* resource
bool inputPanelVisible
QWaylandDestroyListener focusDestroyListener
QString surroundingText
int cursorPosition
int anchorPosition
int absolutePosition
QRect cursorRectangle
QString preferredLanguage
Qt::InputMethodHints hints
}
class qt_text_input_method_v1 {
+QMap~wl_client*, Resource*~ resourceMap()
+void send_enter(wl_resource* resource, wl_resource* surface)
+void send_leave(wl_resource* resource, wl_resource* surface)
+void send_input_direction_changed(wl_resource* resource, int direction)
+void send_locale_changed(wl_resource* resource, QString locale)
}
PluginManager --> QWaylandCompositor : uses
PluginManager --> QWaylandSeat : uses
PluginManager --> QWaylandSurface : uses
PluginManager --> QWaylandQuickItem : uses
PluginManager --> QWaylandTextInputPrivate : uses
PluginManager --> QWaylandTextInputV3Private : uses
PluginManager --> QWaylandQtTextInputMethodPrivate : uses
WlQtTextInputMethodHelper --|> qt_text_input_method_v1
WlQtTextInputMethodHelper --> QWaylandQtTextInputMethodPrivate : manipulates
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The
WlQtTextInputMethodHelper::setFocusCustomimplementation doesauto *base = static_cast<QtWaylandServer::qt_text_input_method_v1*>(d);and laterauto *helper = static_cast<WlQtTextInputMethodHelper*>(base);, butdis aQWaylandQtTextInputMethodPrivate, not aWlQtTextInputMethodHelper, so this downcast is invalid and will be UB at runtime—please remove theWlQtTextInputMethodHelperinheritance/downcast and callsend_enter/other methods directly via the actual type that implements the protocol. - The text input focus wiring relies heavily on private Qt headers (
qwaylandtextinput_p.h,qwaylandqttextinputmethod_p.h,QObjectPrivate::get) and string-basedmetaObject()->className()checks; consider centralizing this logic behind a single helper and adding version/feature guards so that future Qt upgrades do not silently break text input handling.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `WlQtTextInputMethodHelper::setFocusCustom` implementation does `auto *base = static_cast<QtWaylandServer::qt_text_input_method_v1*>(d);` and later `auto *helper = static_cast<WlQtTextInputMethodHelper*>(base);`, but `d` is a `QWaylandQtTextInputMethodPrivate`, not a `WlQtTextInputMethodHelper`, so this downcast is invalid and will be UB at runtime—please remove the `WlQtTextInputMethodHelper` inheritance/downcast and call `send_enter`/other methods directly via the actual type that implements the protocol.
- The text input focus wiring relies heavily on private Qt headers (`qwaylandtextinput_p.h`, `qwaylandqttextinputmethod_p.h`, `QObjectPrivate::get`) and string-based `metaObject()->className()` checks; consider centralizing this logic behind a single helper and adding version/feature guards so that future Qt upgrades do not silently break text input handling.
## Individual Comments
### Comment 1
<location path="panels/dock/pluginmanagerextension.cpp" line_range="80-81" />
<code_context>
+ d->cursorRectangle = QRect();
+ d->preferredLanguage.clear();
+ d->hints = Qt::InputMethodHints();
+ auto *helper = static_cast<WlQtTextInputMethodHelper*>(base);
+ helper->send_enter(d->resource->handle, d->focusedSurface->resource());
+
+ helper->send_input_direction_changed(d->resource->handle, int(qApp->inputMethod()->inputDirection()));
</code_context>
<issue_to_address>
**issue (bug_risk):** The static_cast to WlQtTextInputMethodHelper is undefined behavior because the underlying object is not of that type.
Here `base` is a `QWaylandQtTextInputMethodPrivate` (deriving from `QtWaylandServer::qt_text_input_method_v1`), not a `WlQtTextInputMethodHelper`. Downcasting a base pointer to an unrelated derived type is undefined behavior, even if they share a base. Since you only need `send_enter`, `send_input_direction_changed`, and `send_locale_changed`, call them directly on `base` (or move this logic into a non-inheriting helper) and remove the cast to `WlQtTextInputMethodHelper`.
</issue_to_address>
### Comment 2
<location path="panels/dock/pluginmanagerextension.cpp" line_range="808" />
<code_context>
});
}
+
+void PluginManager::setupTextInputProxy(QWaylandCompositor *compositor)
+{
+ // === IME 事件转发的完整链路说明 ===
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the focus-handling, surfaceEnabled wiring, text-input state reset, and clipboard-sync logic into small helper functions to keep initialize() and the keyboardFocusChanged lambda easier to read and maintain.
You can keep all the behavior while making this code easier to follow by extracting some of the repeated / multi‑purpose logic into small helpers.
### 1. Split the `keyboardFocusChanged` lambda into protocol‑specific helpers
The lambda currently does: selection update, extension iteration, class name dispatch, private casts, and signal wiring. This can be made more readable by extracting the per‑protocol focus handling into small helpers:
```cpp
// In PluginManager (private methods):
void PluginManager::handleTextInputFocus(QObject *ext, QWaylandSurface *newFocus)
{
auto *d = static_cast<QWaylandTextInputPrivate *>(QObjectPrivate::get(ext));
d->setFocus(newFocus);
}
void PluginManager::handleTextInputV3Focus(QObject *ext, QWaylandSurface *newFocus)
{
auto *d = static_cast<QWaylandTextInputV3Private *>(QObjectPrivate::get(ext));
d->setFocus(newFocus);
}
void PluginManager::handleQtTextInputMethodFocus(QObject *ext, QWaylandSurface *newFocus)
{
auto *qtMethodPriv = static_cast<QWaylandQtTextInputMethodPrivate *>(QObjectPrivate::get(ext));
qtMethodPriv->inputPanelVisible = true;
WlQtTextInputMethodHelper::setFocusCustom(qtMethodPriv, newFocus);
}
```
Then `setupTextInputProxy`’s lambda becomes easier to scan:
```cpp
QObject::connect(seat, &QWaylandSeat::keyboardFocusChanged, this,
[this, seat](QWaylandSurface *newFocus, QWaylandSurface *oldFocus) {
Q_UNUSED(oldFocus);
if (newFocus)
newFocus->updateSelection();
const auto extensions = seat->extensions();
for (auto *ext : extensions) {
const QString className = ext->metaObject()->className();
if (className == QStringLiteral("QWaylandTextInput")) {
handleTextInputFocus(ext, newFocus);
} else if (className == QStringLiteral("QWaylandTextInputV3")) {
handleTextInputV3Focus(ext, newFocus);
} else if (className == QStringLiteral("QWaylandQtTextInputMethod")) {
handleQtTextInputMethodFocus(ext, newFocus);
}
connectSurfaceEnabledOnce(ext);
}
});
```
### 2. Deduplicate `surfaceEnabled` connections
You repeat the same `QObject::connect` three times. A small helper keeps the pattern in one place and documents intent:
```cpp
void PluginManager::connectSurfaceEnabledOnce(QObject *ext)
{
QObject::connect(ext, SIGNAL(surfaceEnabled(QWaylandSurface*)),
this, SLOT(onTextInputSurfaceEnabled(QWaylandSurface*)),
Qt::UniqueConnection);
}
```
Used in the loop above, this removes duplication and makes it clear that every text input extension should share the same `surfaceEnabled` behavior.
### 3. Reduce incidental complexity in `WlQtTextInputMethodHelper::setFocusCustom`
`setFocusCustom` mixes: resource lookup, leave/enter protocol messages, state reset, and focus listener handling. You can factor out the “state reset” into a tiny helper to make the control flow clearer:
```cpp
static void resetQtTextInputState(QWaylandQtTextInputMethodPrivate *d)
{
d->surroundingText.clear();
d->cursorPosition = 0;
d->anchorPosition = 0;
d->absolutePosition = 0;
d->cursorRectangle = QRect();
d->preferredLanguage.clear();
d->hints = Qt::InputMethodHints();
}
```
Then:
```cpp
static void setFocusCustom(QWaylandQtTextInputMethodPrivate *d, QWaylandSurface *surface)
{
auto *base = static_cast<QtWaylandServer::qt_text_input_method_v1*>(d);
if (d->focusedSurface == surface)
return;
QtWaylandServer::qt_text_input_method_v1::Resource *newResource = nullptr;
if (surface && surface->client()) {
auto resources = base->resourceMap().values(surface->client()->client());
if (!resources.isEmpty())
newResource = resources.first();
}
if (d->focusedSurface) {
if (d->resource)
base->send_leave(d->resource->handle, d->focusedSurface->resource());
d->focusDestroyListener.reset();
}
d->focusedSurface = surface;
d->resource = newResource;
if (d->resource && d->focusedSurface) {
resetQtTextInputState(d);
auto *helper = static_cast<WlQtTextInputMethodHelper*>(base);
helper->send_enter(d->resource->handle, d->focusedSurface->resource());
helper->send_input_direction_changed(d->resource->handle,
int(qApp->inputMethod()->inputDirection()));
helper->send_locale_changed(d->resource->handle,
qApp->inputMethod()->locale().bcp47Name());
d->focusDestroyListener.listenForDestruction(surface->resource());
if (d->inputPanelVisible && d->enabledSurfaces.values().contains(surface))
qApp->inputMethod()->show();
}
}
```
This keeps the inheritance requirement but makes the method read as higher‑level steps.
### 4. Isolate clipboard setup out of `initialize()`
`initialize()` is becoming an orchestration method; the clipboard logic can be extracted without behavior change:
```cpp
void PluginManager::setupClipboardSync(QWaylandCompositor *compositor)
{
compositor->setRetainedSelectionEnabled(true);
if (auto *clipboard = QGuiApplication::clipboard()) {
QObject::connect(clipboard, &QClipboard::changed, this,
[compositor](QClipboard::Mode mode) {
if (mode == QClipboard::Clipboard || mode == QClipboard::Selection) {
if (const QMimeData *mimeData = QGuiApplication::clipboard()->mimeData(mode))
compositor->overrideSelection(mimeData);
}
});
if (const QMimeData *mimeData = clipboard->mimeData(QClipboard::Clipboard))
compositor->overrideSelection(mimeData);
}
}
```
And in `initialize()`:
```cpp
init(compositor->display(), 1);
// IME manager
auto qtTextInputMethodManager = new QWaylandQtTextInputMethodManager(compositor);
qtTextInputMethodManager->initialize();
// Mouse focus + IME proxy + clipboard
setupMouseFocusListener();
setupTextInputProxy(compositor);
setupClipboardSync(compositor);
```
This keeps `initialize()` at a “wire things together” level, with the detailed logic in dedicated helpers.
</issue_to_address>
### Comment 3
<location path="panels/dock/pluginmanagerextension_p.h" line_range="84" />
<code_context>
//处理鼠标焦点给到相应插件
void setupMouseFocusListener();
+ //处理 IME 输入代理:将插件进程的 text input 请求透传给外层 compositor
+ void setupTextInputProxy(QWaylandCompositor *compositor);
Q_SIGNALS:
</code_context>
<issue_to_address>
**issue (complexity):** Consider encapsulating the IME/text-input helper implementation in the cpp file and visually grouping the related methods in the header to keep PluginManager’s interface thinner and more focused.
You can keep the new behavior while reducing header-level complexity by pushing the protocol-specific IME helper details into the cpp and lightly grouping the new methods.
### 1. Hide `WlQtTextInputMethodHelper` from the header
If `WlQtTextInputMethodHelper` doesn’t need to be referenced from other translation units, move its definition into an anonymous namespace in the cpp and keep only a minimal handle in the header (ideally in a private impl struct).
**Header (keep thin):**
```cpp
// pluginmanager_p.h
class PluginManagerPrivate
{
public:
// ...
// opaque pointer to IME helper, defined in cpp
std::unique_ptr<void> textInputHelper;
};
```
**Cpp (keep details local and protocol-specific):**
```cpp
// pluginmanager.cpp
namespace {
// fully define the helper here, using private Qt/Wayland API as needed
struct WlQtTextInputMethodHelper
{
void setupTextInputProxy(QWaylandCompositor *compositor);
void onTextInputSurfaceEnabled(QWaylandSurface *surface);
// ...
};
} // anonymous namespace
PluginManagerPrivate::PluginManagerPrivate()
: textInputHelper(std::make_unique<WlQtTextInputMethodHelper>())
{
}
```
Then in methods:
```cpp
void PluginManager::setupTextInputProxy(QWaylandCompositor *compositor)
{
auto *helper = static_cast<WlQtTextInputMethodHelper*>(d->textInputHelper.get());
helper->setupTextInputProxy(compositor);
}
void PluginManager::onTextInputSurfaceEnabled(QWaylandSurface *surface)
{
auto *helper = static_cast<WlQtTextInputMethodHelper*>(d->textInputHelper.get());
helper->onTextInputSurfaceEnabled(surface);
}
```
This keeps the private header free from protocol-heavy types and reduces mental load for anyone reading `pluginmanager_p.h`.
### 2. Group the IME-related methods in the header
Make the IME responsibility visually distinct from the rest of `PluginManager` to avoid the “god object” feel and clarify intent:
```cpp
class PluginManager : public QWaylandCompositorExtensionTemplate<PluginManager>
{
Q_OBJECT
public:
// ...
void setEmbedPanelMinHeight(int height);
QSize dockSize() const;
void setDockSize(const QSize &newDockSize);
void removePluginSurface(PluginSurface *plugin);
// Mouse focus management
void setupMouseFocusListener();
// IME / text input proxying: forward plugin text input to outer compositor
void setupTextInputProxy(QWaylandCompositor *compositor);
Q_SIGNALS:
// ...
private Q_SLOTS:
// Theme / appearance
void onFontChanged();
void onActiveColorChanged();
void onThemeChanged();
// IME / text input
void onTextInputSurfaceEnabled(QWaylandSurface *surface);
```
This keeps responsibilities discoverable and makes future IME-related additions less likely to blur with unrelated behavior, without changing any functionality you’ve introduced.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
dec49f5 to
b36c7aa
Compare
0d3c169 to
e01b6ff
Compare
This commit enables Chinese input methods (like fcitx) to correctly interact with Wayland dock plugin windows. The fix introduces an explicit text input proxy routing: 1. Since QWaylandSeat::setKeyboardFocus() does not update text input protocols automatically, we manually assign the focused surface to all text input global extensions (QWaylandTextInput/V3 & QWaylandQtTextInputMethod) via private APIs. 2. In QWaylandQtTextInputMethod, the d->resource was missing during continuous window focus change, breaking client messages like update_cursor_rectangle. We intercept focus management using a custom WlQtTextInputMethodHelper. 3. Connecting surfaceEnabled signal to automatically invoke forceActiveFocus() on the QWaylandQuickItem rendering the surface frame. This is crucial because Qt requires active focus to correctly route QInputMethodEvent downwards to the client. Log: support wayland text input on plugin popup windows Pms: BUG-323547
e01b6ff to
3966919
Compare
To fix pasting text in Wayland plugin windows, this commit ensures that nested Wayland clients can correctly receive host clipboard contents. Wayland compositor (dde-shell) does not automatically synchronize the host system's clipboard to its nested clients. This caused plugin popup windows to be unable to paste text that was copied from outside applications. This fix enables retained selection in QWaylandCompositor and explicitly listens to QClipboard::changed from the host environment to forward the QMimeData to the compositor via overrideSelection(). Log: fix pasting text in Wayland plugin windows Pms: BUG-329435
3966919 to
ff8f1fc
Compare
deepin pr auto review这段代码主要实现了在 Wayland Compositor 中处理输入法(IME)和剪贴板的代理逻辑,特别是针对 Qt TextInput 协议的焦点管理和事件转发。以下是对代码的详细审查和改进建议: 1. 语法与逻辑审查1.1 头文件包含
1.2 焦点管理逻辑
1.3 信号连接
2. 代码质量2.1 注释
2.2 魔法值
2.3 错误处理
3. 性能优化3.1 遍历扩展
3.2 信号连接
4. 安全性4.1 私有 API 访问
4.2 剪贴板同步
5. 其他建议5.1 内存管理
5.2 测试覆盖
改进后的代码示例void PluginManager::setupTextInputProxy(QWaylandCompositor *compositor)
{
QWaylandSeat *seat = compositor->defaultSeat();
if (!seat)
return;
// 缓存扩展对象
static QHash<QByteArray, QObject*> extensionCache;
if (extensionCache.isEmpty()) {
const auto extensions = seat->extensions();
for (auto *ext : extensions) {
extensionCache.insert(ext->metaObject()->className(), ext);
}
}
QObject::connect(seat, &QWaylandSeat::keyboardFocusChanged, this, [this, seat]
(QWaylandSurface *newFocus, QWaylandSurface *oldFocus) {
Q_UNUSED(oldFocus);
if (newFocus) {
newFocus->updateSelection();
}
// 使用缓存的扩展对象
for (auto it = extensionCache.constBegin(); it != extensionCache.constEnd(); ++it) {
const QByteArray &className = it.key();
QObject *ext = it.value();
if (className == "QWaylandTextInput") {
auto *d = dynamic_cast<QWaylandTextInputPrivate*>(QObjectPrivate::get(ext));
if (d) {
d->setFocus(newFocus);
QObject::connect(ext, &QWaylandTextInput::surfaceEnabled,
this, &PluginManager::onTextInputSurfaceEnabled,
Qt::UniqueConnection);
}
} else if (className == "QWaylandTextInputV3") {
// 类似处理...
} else if (className == "QWaylandQtTextInputMethod") {
// 类似处理...
}
}
});
}总结这段代码功能完整,但存在以下主要问题:
建议逐步重构,减少对私有 API 的依赖,优化性能,并增强健壮性。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: tsic404, yixinshark The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
Summary by Sourcery
Improve Wayland dock plugin text input handling and clipboard integration to fix IME typing and paste behavior in plugins.
New Features:
Bug Fixes:
Enhancements: